Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Polish Dropship Weapons UI #5298

Merged
merged 15 commits into from
Jan 6, 2024
Merged

Polish Dropship Weapons UI #5298

merged 15 commits into from
Jan 6, 2024

Conversation

Drulikar
Copy link
Contributor

@Drulikar Drulikar commented Dec 25, 2023 •
edited
Loading

About the pull request

This PR is a follow up to #4812 polishing minor issues that were discovered but not addressed in that PR.

  • Fire missions can now be scrolled in target acquisition
  • The target selection in equipment view is now the same as other panels (and can be scrolled now too)
  • Fixed deploying equipment like the spotlight and sentry
  • Partially fixed camera view for sentry (map size doesn't always get set correctly some reason)
  • Fixed the extra scrolling of fire missions
  • Fixed the scrolling of target selection being a fixed quantity of 10 targets
  • Improved menuing for the fire mission and strike sub menus
  • Fixed strike ready messaging
  • Map and Cam buttons are now mutually exclusive to one another (would just break the byond UI)
  • Camera now resets when a CAS flare is destroyed (unless fire mission is underway)
  • Camera now resets when sentry undeploys (if it was being used)
  • Tweaked some button placements
  • Fixed medevac layout
  • Fixed direct fire (strike & equipment)
  • Fixed camera views not handling /datum/component/overlay_lighting correctly (fixed by using TILE_BOUND planes; but will note it won't work completely correct if on a byond version prior to 515.1609 because of https://www.byond.com/forum/post/2873835)
  • Fixed some hard deletes in /client/proc/clear_map based on Harddeletes: Accident edition tgstation/tgstation#61562
  • Renamed nvgon and nvgoff to NV-ON and NV-OFF

Explain why it's good for the game

Fixes issues such as (but not limited to):
target
image

Testing Photographs and Procedure

Screenshots & Videos

image
image
image
light

Changelog

🆑 Drathek
ui: Polished various aspects of the new dropship weapons UI
fix: Fixed CAS direct firing
fix: Fixed Medevac buttons not moving the dropship (still currently requires manual winch)
fix: Fixed camera_manager resizing the view incorrectly because of overlay_lighting
refactor: Ported some hard delete fixes for maps.
/:cl:

@Drulikar Drulikar requested a review from fira as a code owner December 25, 2023 10:21
@github-actions github-actions bot added the UI deletes nanoui/html label Dec 25, 2023
@SabreML

This comment was marked as off-topic.

Sign in to view
fira
fira reviewed Dec 26, 2023
View reviewed changes
Copy link
Member

@fira fira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - is that TM worth?

code/game/cas_manager/datums/cas_fire_envelope.dm Show resolved Hide resolved
@eMiiXi
Copy link

eMiiXi commented Dec 26, 2023

I was told to comment on the actual PR to report bugs/issues.
With this current PR its impossible to do direct fire on target, so it makes Napalm missile, and dropping turrets impossible.
I have extensively tried to go into Equip > Select the weapon i want to direct fire and the target > Press FIRE. Nothing happens.
I also went into target mode > Strike > Weapon > Select a missile that its currently loaded and i want to fire on target > Press FIRE. Nothing happens. Other PO's have also reported the same issue and being unable to use direct fire at all, and unable to drop turrets or napalm because of this. The only way to currently do direct fire is to do a entire fire mission for it, but some things like Napalm can only be fired by direct, and are restricted from being added into a fire mission.

Also, when using medevac, you can click Equip to go back to the main selection of installed modules/guns, but when you do fulton the equip button is not there, just minor QoL, but makes it so you have to full back then go into equip again.

@TTaxer TTaxer mentioned this pull request Dec 26, 2023
Closed
3 tasks
@Drulikar Drulikar marked this pull request as draft December 26, 2023 23:35
@Drulikar Drulikar marked this pull request as ready for review December 27, 2023 01:18
@Drulikar
Copy link
Contributor Author

I was told to comment on the actual PR to report bugs/issues. With this current PR its impossible to do direct fire on target, so it makes Napalm missile, and dropping turrets impossible. I have extensively tried to go into Equip > Select the weapon i want to direct fire and the target > Press FIRE. Nothing happens. I also went into target mode > Strike > Weapon > Select a missile that its currently loaded and i want to fire on target > Press FIRE. Nothing happens. Other PO's have also reported the same issue and being unable to use direct fire at all, and unable to drop turrets or napalm because of this. The only way to currently do direct fire is to do a entire fire mission for it, but some things like Napalm can only be fired by direct, and are restricted from being added into a fire mission.

Also, when using medevac, you can click Equip to go back to the main selection of installed modules/guns, but when you do fulton the equip button is not there, just minor QoL, but makes it so you have to full back then go into equip again.

Thanks for the report. All that should be addressed now too. I'll note however the medevac interaction will just move the dropship over the stretcher and set the camera; hoisting still requires manual interaction.

@Drulikar Drulikar added Fix Fix one bug, make ten more Testmerge Candidate we'll test this while you're asleep and the server has 10 players labels Dec 27, 2023
@professionallyretarded professionallyretarded mentioned this pull request Dec 31, 2023
Closed
3 tasks
@deathrobotpunch deathrobotpunch mentioned this pull request Dec 31, 2023
Closed
3 tasks
@Superwelder Superwelder mentioned this pull request Jan 1, 2024
Open
3 tasks
@Accoint Accoint mentioned this pull request Jan 1, 2024
Open
3 tasks
@Red-byte3D Red-byte3D mentioned this pull request Jan 2, 2024
Open
3 tasks
@private-tristan private-tristan mentioned this pull request Jan 3, 2024
Closed
3 tasks
@Zonespace27 Zonespace27 mentioned this pull request Jan 3, 2024
Open
3 tasks
This was referenced Jan 4, 2024
Open
Closed
@Doom-external Doom-external mentioned this pull request Jan 5, 2024
Closed
3 tasks
@harryob harryob added this pull request to the merge queue Jan 6, 2024
Merged via the queue into cmss13-devs:master with commit b6ed599 Jan 6, 2024
26 checks passed
cm13-github added a commit that referenced this pull request Jan 6, 2024
@cm13-github
Automatic changelog for PR #5298 [ci skip]
ba28bfe
@Drulikar Drulikar deleted the Polish_CAS_UI branch January 8, 2024 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@harryob harryob harryob approved these changes

@spookydonut spookydonut spookydonut approved these changes

@fira fira fira approved these changes

Assignees
No one assigned
Labels
Fix Fix one bug, make ten more Refactor Make the code harder to read Testmerge Candidate we'll test this while you're asleep and the server has 10 players UI deletes nanoui/html
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@Drulikar @SabreML @eMiiXi @BartDrown @fira @spookydonut @harryob